CloudStack Volume support with ONTAP storage#13053
CloudStack Volume support with ONTAP storage#13053rajiv-jain-netapp wants to merge 93 commits intoapache:mainfrom
Conversation
… added EOF fixes + correcting license header
Initial primary storage pool plugin skeleton
…POJOs" This reverts commit fe0f752.
…POJOs" This reverts commit 28faca1.
Feignconfiguration and volume feignClient along with desired POJOs with cstack 28
* CSTACKEX-29 Cluster, SVM and Aggr Feign Client * CSTACKEX-29 Change the endpoint method name in feign client * CSTACKEX-29 Make the alignment proper * CSTACKEX-29 Added License Info * CSTACKEX-29 Resolve Review Comments * CSTACKEX-29 Remove Component Annotation from datastoredriverclass * CSTACKEX-29 Resolve Style check issues * CSTACKEX-29 Resolve ALL Style issues * CSTACKEX-29 Resolve Precommits Issues * CSTACKEX-29 Added Method comments and change the ontap response class name --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-31 NAS and Job Feign Client and POJOs * CSTACKEX-31 Fixed Checks Issues * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Added Aggr and size to volume model * CSTACKEX-31 Change the export policy endpoint path * CSTACKEX-31 Fixed check styles --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-30 SAN Feign Client * CSTACKEX-30 Fixed check style issues * CSTACKEX-30 Fixed review comments --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-7: ONTAP Primary storage pool --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>
CSTACKEX-34: Upgrade to framework classes design
* CSTACKEX-35 Create Async * CSTACKEX-35 Added Null and empty check * CSTACKEX-35 Resolved review comments * CSTACKEX-35 Removed Type Casting for logger --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-1: Feign changes and fixes for getting storage pool creation to work * CSTACKEX-01: Create Primary Storage pool changes with working code * CSTACKEX-01: Addressed all review comments and updated some code * CSTACKEX-01: Made some changes to fix some errors seen during testing * CSTACKEX-01: Addressed additional comments --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>
NFS3 protocol specific snapshot workflows
Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
… any storage if any vm gets deleted Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
…th not a bootable error in console (#43) Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
… takes less than 30s copy will be a dummy VM (#42) ### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1451" height="117" alt="image" src="https://github.com/user-attachments/assets/3af9c3b4-e834-42f9-925d-6e87028fe3bf" /> <img width="1491" height="574" alt="image" src="https://github.com/user-attachments/assets/ac2a39c2-d32f-4c22-89d1-261a2e15bdd7" /> ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
) ### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1044" height="723" alt="image" src="https://github.com/user-attachments/assets/5edcd661-7b6a-4b5f-adb1-efc34faf3c76" /> <img width="1446" height="175" alt="image" src="https://github.com/user-attachments/assets/60cc9f05-d096-45d6-8c92-d1a7ce49b42a" /> ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
There was a problem hiding this comment.
Pull request overview
This PR adds NetApp ONTAP as a managed primary storage provider, including UI wiring for pool creation and backend support for CloudStack volume workflows over NFS3 and iSCSI, plus VM/volume snapshot integration for KVM.
Changes:
- UI: adds NetApp ONTAP provider inputs (IP/username/password/SVM/protocol) and provider-specific parameter mapping.
- Backend: introduces ONTAP storage strategies (NAS/SAN), lifecycle attach/delete behavior, and Feign clients/models for snapshot + file/LUN restore workflows.
- KVM/engine: adjusts iSCSI handling and VM snapshot behavior to integrate ONTAP-specific snapshot strategy/limitations.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/infra/zone/ZoneWizardLaunchZone.vue | Adds NetApp ONTAP provider handling + details mapping during zone wizard primary storage creation |
| ui/src/views/infra/zone/ZoneWizardAddResources.vue | Adds ONTAP-specific form fields and protocol choices in zone wizard |
| ui/src/views/infra/zone/StaticInputsForm.vue | Adds numeric-format validation helper used by static forms |
| ui/src/views/infra/AddPrimaryStorage.vue | Adds ONTAP provider form fields, validation rules, and API param mapping |
| ui/public/locales/en.json | Adds English i18n strings for ONTAP fields/tooltips |
| server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java | Adds ONTAP-specific error message when memory snapshots requested without a suitable strategy |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java | Adds unit tests for UnifiedNASStrategy behaviors |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java | Adds unit tests for StorageStrategy connect/volume/network methods |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java | Extends lifecycle tests for init/attach cluster/zone flows |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java | Adds driver tests for create/delete/grant/revoke access flows |
| plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml | Registers ONTAP VM snapshot strategy bean |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java | Adds ONTAP helpers for auth headers, naming, and protocol-specific volume request building |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java | Updates provider naming and adds constants for snapshot/mount options |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java | Extends request/response model to support snapshot workflows (flexvol UUID, destination path) |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java | Refactors access-group request to use storagePoolId instead of PrimaryDataStoreInfo |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java | Implements iSCSI LUN CRUD, igroup management, LUN mapping helpers, and snapshot revert via CLI API |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java | Implements NFS volume handling via host commands + export policy management + snapshot revert via CLI API |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java | Centralizes Feign client creation and adds snapshot job polling + snapshot feign accessors |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java | Adds SAN helper contract for ensuring LUN mappings and initiator validation |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/StorageProviderFactory.java | Selects unified NAS/SAN strategy by protocol and injects components |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java | Passes pool details to agent on host connect and updates pool host refs/capacity |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | Refactors initialization inputs, forces NFSv3 mount options, and adds attach/delete logic updates |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java | Adds concise Volume model used by snapshot responses |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/SnapshotFileRestoreRequest.java | Adds request model for snapshot file restore API |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/OntapStorage.java | Refactors ONTAP connection model to use storageIP and removes disaggregated flag |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunRestoreRequest.java | Adds request model for LUN restore API |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FlexVolSnapshot.java | Adds model for FlexVolume-level snapshots |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileInfo.java | Adjusts file model fields for NAS operations |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileClone.java | Adds request model for ONTAP file clone |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/CliSnapshotRestoreRequest.java | Adds CLI passthrough restore-file request model |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java | Adds Feign client for FlexVol snapshot and restore APIs (including CLI passthrough) |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java | Adds Feign method for LUN restore |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java | Updates file GET to include metadata and adds file clone endpoint |
| plugins/storage/volume/ontap/pom.xml | Adds dependencies and surefire argLine for tests |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java | Makes iSCSI login/create idempotent, rescans sessions, avoids logging out when other LUNs exist, adds flush/sync |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java | Improves error handling and consolidates cleanup logic for snapshot deltas |
| engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java | Updates managed template copy flow to use iSCSI/LUN info available post-grantAccess for ONTAP |
| engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java | Excludes ONTAP managed pools from the generic KVM file-based VM snapshot strategy and improves failure messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Delete access groups associated with this storage pool | ||
| storageStrategy.deleteAccessGroup(accessGroup); | ||
| logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName()); |
There was a problem hiding this comment.
deleteDataStore calls storageStrategy.deleteAccessGroup(accessGroup) but the AccessGroup instance only has storagePoolId set; hostsToConnect is never populated. The SAN implementation (UnifiedSANStrategy.deleteAccessGroup) iterates accessGroup.getHostsToConnect() and will throw at runtime for iSCSI pools. Either populate hostsToConnect (e.g., from the cluster/zone scope or storage_pool_host_ref) before calling deleteAccessGroup, or make SAN deleteAccessGroup handle a null/empty host list safely (and/or skip SAN access-group deletion here if it cannot be determined).
| // Delete access groups associated with this storage pool | |
| storageStrategy.deleteAccessGroup(accessGroup); | |
| logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName()); | |
| // Delete access groups associated with this storage pool only when host information is available. | |
| // Some SAN strategy implementations iterate hostsToConnect and can fail at runtime if it is null. | |
| if (accessGroup.getHostsToConnect() == null || accessGroup.getHostsToConnect().isEmpty()) { | |
| logger.warn("deleteDataStore: Skipping access-group deletion for storage pool '{}' because hostsToConnect is not populated", | |
| storagePool.getName()); | |
| } else { | |
| storageStrategy.deleteAccessGroup(accessGroup); | |
| logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName()); | |
| } |
There was a problem hiding this comment.
we identified this as bug and fixed it in pushed as part of this PR as well.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13053 +/- ##
===========================================
Coverage 17.95% 17.96%
- Complexity 16503 16579 +76
===========================================
Files 6019 6036 +17
Lines 540748 542698 +1950
Branches 66255 66638 +383
===========================================
+ Hits 97084 97486 +402
- Misses 432723 434213 +1490
- Partials 10941 10999 +58
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Co-authored-by: Sandeep Locharla sandeep.locharla@netapp.com
Co-authored-by: Piyush Srivastava piyush5@netapp.com
Co-authored-by: Surya Gupta suryag@netapp.com
This PR...
1. memory snapshot
2. user input for quicing option during snapshot creation
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Testing Done:
How did you try to break this feature and the system with this change?